Conversation
Summary of ChangesHello @gemdev111, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new 'Earn' feature to the application, enabling users to participate in yield-generating activities. This involved a significant refactoring of the existing 'Staking' module into a more generalized 'Earn' module, along with extensive updates across the UI, view models, data models, persistence layer, and transaction signing logic to support the new functionality. The changes aim to provide users with a comprehensive interface for managing their earn positions and interacting with various yield providers. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces the 'Earn' feature, providing a unified interface for yield-generating activities. The implementation includes new scenes, view models, and service layers, along with necessary database migrations and signing logic for EVM chains. While the architecture is consistent with the rest of the project, there are critical safety issues in the database migrations and potential runtime crashes in the view models that should be addressed.
| } | ||
|
|
||
| migrator.registerMigration("Add earn support") { db in | ||
| try? db.alter(table: AssetRecord.databaseTableName) { |
There was a problem hiding this comment.
Using try? in database migrations is highly discouraged. If a migration fails (e.g., due to a schema conflict or database lock), the failure is silently ignored, leaving the database in an inconsistent state. Subsequent code that relies on these schema changes will likely crash or behave unpredictably. Use try to ensure that migration errors are caught and handled by the DatabaseMigrator. This pattern should be corrected on lines 397, 404, and 411 as well.
| try? db.alter(table: AssetRecord.databaseTableName) { | |
| try db.alter(table: AssetRecord.databaseTableName) { |
|
|
||
| var depositDestination: AmountInput { | ||
| AmountInput( | ||
| type: .earn(.deposit(provider: providers.first!)), |
There was a problem hiding this comment.
Force unwrapping providers.first! is unsafe. Although the UI visibility is currently controlled by showDeposit, this property is public and could be accessed by other components or tests when providers is empty, causing a crash. Consider making depositDestination optional or providing a safe fallback.
| func fetch() async { | ||
| viewState = .loading | ||
| do { | ||
| let address = (try? wallet.account(for: asset.id.chain))?.address ?? "" |
There was a problem hiding this comment.
If the wallet does not contain an account for the asset's chain, address will be an empty string. Calling earnService.update with an empty address is likely to result in an invalid API request or incorrect data fetching. It is safer to guard against a missing account and return early.
| let address = (try? wallet.account(for: asset.id.chain))?.address ?? "" | |
| guard let address = (try? wallet.account(for: asset.id.chain))?.address else { return } |
6760b81 to
9f40288
Compare
| // Copyright (c). Gem Wallet. All rights reserved. | ||
|
|
||
| public enum DelegationDetailActionType: Hashable, Identifiable { | ||
| public var id: Self { self } |
| .asset(assetImage: AssetViewModel(asset: asset).assetImage) | ||
| } | ||
|
|
||
| public var availableActions: [DelegationDetailActionType] { |
| self.delegation = delegation | ||
| self.currencyCode = currencyCode | ||
| self.asset = asset ?? delegation.base.assetId.chain.asset |
There was a problem hiding this comment.
| self.asset = asset ?? delegation.base.assetId.chain.asset | |
| self.asset = asset |
|
|
||
| public init( | ||
| delegation: Delegation, | ||
| asset: Asset? = nil, |
There was a problem hiding this comment.
| asset: Asset? = nil, | |
| asset: Asset, |
| } | ||
|
|
||
| var title: String { Localized.Common.earn } | ||
| var assetTitle: String { "\(asset.symbol) (\(asset.chain.asset.name))" } |
There was a problem hiding this comment.
this should be in AssetViewModel
|
|
||
| var depositDestination: AmountInput { | ||
| AmountInput( | ||
| type: .earn(.deposit(providers.first!)), |
There was a problem hiding this comment.
| type: .earn(.deposit(providers.first!)), | |
| type: .earn(.deposit(providers.first)), |
| } | ||
|
|
||
| func makeTransferData(value: BigInt) async throws -> TransferData { | ||
| let address = (try? wallet.account(for: asset.chain).address) ?? "" |
There was a problem hiding this comment.
| let address = (try? wallet.account(for: asset.chain).address) ?? "" | |
| let address = try wallet.account(for: asset.chain).address) |
|
|
||
| var earnBalanceText: String { | ||
| let balance = assetData.balance.earn | ||
| guard balance > 0 else { return "0" } |
There was a problem hiding this comment.
| guard balance > 0 else { return "0" } | |
| guard balance > 0 else { return .zero } |
| @@ -0,0 +1,9 @@ | |||
| // Copyright (c). Gem Wallet. All rights reserved. | |||
|
|
|||
| public enum DelegationActionType: Hashable, Identifiable { | |||
There was a problem hiding this comment.
DelegationActionType.swift
| } | ||
|
|
||
| public var showManage: Bool { | ||
| !availableActions.isEmpty |
There was a problem hiding this comment.
| !availableActions.isEmpty | |
| availableActions.isNotEmpty |
|
|
||
| var aprTitle: String { Localized.Stake.apr("") } | ||
| var aprValue: String { | ||
| let apr = providers.first.map(\.apr).flatMap { $0 > 0 ? $0 : nil } |
There was a problem hiding this comment.
split to var private apr: Double?
| .getStakeBalance(for: address) | ||
| } | ||
|
|
||
| func getEarnBalance( |
| case panora | ||
| case thorchain | ||
| case jupiter | ||
| case okx |
| )) | ||
| } | ||
|
|
||
| func signEarn(input: SignerInput, privateKey: Data) throws -> [String] { |
There was a problem hiding this comment.
reuse code with signSwap
|
|
||
| import Foundation | ||
|
|
||
| public struct EarnTransaction: Codable, Sendable { |
|
|
||
| import Foundation | ||
|
|
||
| public struct EarnData: Codable, Equatable, Hashable, Sendable { |
There was a problem hiding this comment.
| public struct EarnData: Codable, Equatable, Hashable, Sendable { | |
| public struct StakeData: Codable, Equatable, Hashable, Sendable { |
or similar, that's reused for Stake and Earn
There was a problem hiding this comment.
use StakeData, data and to should not be optional.
Move AmountInputAction to Primitives Add providerType to store and earn DB queries
Update imports for Staking to Earn rename Add earn FFI mappings Refactor AmountType with StakeAmountType Add earn transaction types and exhaustiveness fixes Add earn scenes and navigation Add earn balance and metadata support Add earn button to asset detail screen Add earn asset handling and validator UI Introduce assetRequest and assetData to EarnSceneViewModel and observe asset updates in EarnNavigationView to surface asset metadata (e.g. earn APR). Use assetData.metadata.earnApr as a fallback when computing APR. Rename showEarn -> showEarnBalance and update related computed properties (showBalances, showEarnButton) and inline the stake header link in AssetScene while adding accessibility identifiers for stake and earn. Update Validator views: map earn providers to YieldProvider display name in StakeValidatorViewModel and make ValidatorImageView public to show provider images (earn) or validator images (stake). Also update AmountScene to display the earn provider in the amount input section. Regenerate typeshare types and fix callers Add getEarnData gateway method Move RecipientNavigationView to Gem target Add EarnData to earn transaction flow Add loading indicator to amount toolbar
Refactor StakeDelegation to DelegationViewModel Replace the old StakeDelegationViewModel/view/header with a unified DelegationViewModel and DelegationView. Key changes: - Deleted StakeDelegationViewModel, StakeDelegationView, and ValidatorHeaderViewModel. - Introduced DelegationViewModel changes (adds ExplorerService, validatorUrl) and new unit tests (DelegationViewModelTests). - Updated StakeScene to use DelegationView and model.navigationDestination(for:) instead of the old types. - StakeSceneViewModel now accepts a currencyCode, maps delegations to DelegationViewModel, and exposes navigationDestination(for:) helper. - StakeDetailSceneViewModel updated to use DelegationViewModel and DelegationHeaderViewModel. - ViewModelFactory now supplies Preferences.standard.currency when building stake view models. - Submodule 'core' reference bumped. These changes consolidate delegation presentation and navigation logic, centralize currency handling, and wire explorer URLs for validators. Introduce ValidatorViewModel and refactor validator logic Centralize validator representation by adding ValidatorViewModel and moving name/image/url/apr logic into it. Remove legacy view models and helpers (StakeValidatorViewModel, YieldProviderViewModel, DelegationHeaderViewModel, DelegationState+Staking) and update DelegationViewModel to use ValidatorViewModel (also conforming DelegationViewModel to HeaderViewModel). Update views and view models (ValidatorImageView, ValidatorSelectionView, ValidatorView, StakeValidatorsViewModel, EarnDetail/StakeDetail scenes) to construct and consume ValidatorViewModel. Rename and update tests accordingly (StakeValidatorViewModelTests -> ValidatorViewModelTests). Misc: small formatting/import adjustments and use of DelegationStateViewModel for state title.
Rename StakeDetailScene to DelegationDetailScene and introduce DelegationDetailSceneViewModel to unify stake and earn provider handling. Add DelegationDetailActionType and consolidate availableActions/action handling into the new view model (ForEach-driven action list). Remove legacy EarnDetailScene, StakeDetailSceneViewModel and EarnDetailSceneViewModel. Update DelegationStateViewModel to expose color and textStyle and refactor DelegationViewModel to use it. Wire navigation and factory: update EarnNavigationView/StakeNavigationView and ViewModelFactory to use delegationDetailScene and pass validators. Adjust tests and mocks to the new DelegationDetailSceneViewModel.
Replace request/value pairs with ObservableQuery in EarnSceneViewModel (assetQuery, positionsQuery, providersQuery) and expose computed properties (assetData, positions, providers). Update initial query values. Update UI bindings: EarnNavigationView now uses .bindQuery(...) and removes an unused Store import. Rename StakeDetailSceneViewModelTests to DelegationDetailSceneViewModelTests. Update DelegationDetailScene to call model.onSelectAction(...) instead of performAction. Add earn-related defaults to AssetData (isEarnEnabled, earnApr).
Add EarnService support across the transaction system: handle .earn transaction types in TransactionFactory, propagate earnService through ServicesFactory into TransactionStateService, and wire it into TransactionPostProcessingService to call earnService.update for earnDeposit/earnWithdraw. Update package manifest to include EarnService and test dependencies, and extend the TransactionStateService test kit with an EarnService.mock provider. These changes enable processing and post-processing of earn-related transactions.
Expose and propagate 'earn' balances throughout the stack: added ChainBalanceable.getEarnBalance and a default empty implementation; implemented getEarnBalance in GatewayService and GatewayChainService to forward and map gateway responses; updated ChainServiceMock to stub earn balances. BalanceFetcher can now request earn balances, and BalanceService schedules and handles earn balance updates (added updateEarnBalance and a task to fetch/update earn balances, mapping via .earnChange). These changes enable retrieval and processing of earn/yield balances alongside existing coin/stake/token balances.
Simplify and tighten Earn-related APIs: remove the chain parameter from EarnService.getEarnData and its protocol, add an assetIds parameter to GatewayService.earnPositions, and make GatewayService.earnProviders synchronous and non-throwing (using compactMap). Update EarnService to fetch providers (non-throwing) then request positions for the specific asset, and adjust callers/test mocks (AmountEarnViewModel and MockEarnService) to match the new signatures. Also updates the core submodule commit reference.
- BalanceService: guard on earnAmount > 0 in updateEarnBalance to skip chain calls when not needed - EarnService: write BalanceRecord.earn from positions to bootstrap earn balance - EarnService: extract getEarnUpdateBalance for clean separation - Update ServicesFactory and TestKit for EarnService balanceStore dependency
Replace scattered stateText/stateStyle accessors with a single stateModel property that returns DelegationStateViewModel. Updated DelegationViewModel and DelegationDetailSceneViewModel to expose stateModel, and adjusted DelegationView and DelegationDetailScene to use stateModel.title and stateModel.textStyle. This centralizes state presentation logic and removes duplicated mapping of state to title/style.
Replace silent optional fallback that returned an empty address with a throwing account lookup in EarnSceneViewModel.fetch and AmountEarnViewModel.makeTransferData. This removes the previous (try? ...)?.address ?? "" pattern so errors from wallet.account(for:) surface instead of allowing an empty address to be used, enabling proper error handling and preventing invalid requests.
Refactor DelegationDetail types and files to DelegationScene: rename scene, view model, and action enum (DelegationDetailScene -> DelegationScene, DelegationDetailSceneViewModel -> DelegationSceneViewModel, DelegationDetailActionType -> DelegationActionType). Update usages in navigation and ViewModelFactory, adjust internal properties (chain -> stakeChain, recommendedCurrentValidator -> recommendedValidator) and related logic. Add a new StakeTestKit target to Package.swift and include a DelegationSceneViewModel test helper and updated tests; remove the old DelegationDetail tests. Also update the core submodule reference.
Rename StakeValidatorsScene, StakeValidatorsViewModel, and StakeValidatorsType to ValidatorSelectScene, ValidatorSelectSceneViewModel, and ValidatorSelectType respectively. Update related call sites and API: replace model/state types, initializer parameters, and the stakeValidatorsType property with validatorSelectType (used in AmountNavigationView and AmountStakeViewModel) to match the new names.
Avoid force-unwrapping providers.first in EarnSceneViewModel by making depositDestination return an optional AmountInput and returning nil when there are no providers. Update TransferDataViewModel to use Localized.Wallet.deposit for the earn-deposit case instead of Localized.Transfer.Stake.title to display the correct label. Changes improve safety and correct the UI string.
Make Asset a required parameter of DelegationViewModel (remove optional fallback to delegation.base.assetId.chain.asset) and update all call sites accordingly. Also switch EarnSceneViewModel to use AssetViewModel.title for asset display. Tests and TestKit updated to pass the asset where needed. This enforces explicit asset injection and centralizes asset title formatting.
- Create reusable AprViewModel in PrimitivesComponents for consistent APR formatting (green styled value via TextValue) - Replace duplicated APR formatting logic across StakeSceneViewModel, EarnSceneViewModel, DelegationSceneViewModel, and ValidatorViewModel with AprViewModel - Consolidate stakeApr/earnApr in AssetDataViewModel into apr(for: StakeProviderType) - Remove redundant aprText wrapper properties — callers use aprModel.text directly
Rename AssetImageTitleView to AssetPreviewView and switch consumers to use AssetViewModel instead of the removed AssetImageTitleViewModel. Add subtitleSymbol to AssetViewModel (hides symbol when equal to name), update Receive/Recipient scenes to use AssetPreviewView(model: assetModel), and adjust Recipient/Receive view models accordingly. Update tests to cover subtitleSymbol and networkFullName expectations.
Introduce provider-based handling for stake and earn balances across the feature: replace separate showStakedBalance/showEarnBalance with showProviderBalance(for:), add balanceTitle(for:) and aprModel(for:) helpers, and update AssetScene to use these methods for titles/subtitles. Move showStakedBalanceTypes to a private static and make onSelectBuy/onSelectSwap private. Update BalanceViewModel to provide balanceTextWithSymbol(for:) (including handling .earn) and forward that from AssetDataViewModel. Update test fixtures: add earn to Balance testkit, add AssetDataViewModel testkit mock, and extend unit tests to cover provider visibility, titles and balance formatting.
Rename Features/Stake/Sources/Types/DelegationAction.swift to DelegationActionType.swift and update usages. Replace `!...isEmpty` checks with the `isNotEmpty` property in DelegationSceneViewModel and StakeSceneViewModel for clarity. Also reformat a BalanceFetcher method signature into a single line (no behavior changes).
Remove Earn-specific balance persistence and the BalanceStore dependency from EarnService. Deleted BalanceStore.hasEarnBalance and the EarnService.balanceStore property and related updateEarnBalance logic; updated EarnService initializer and tests accordingly. Adjusted ServicesFactory to stop passing balanceStore into EarnService. Removed Formatters import and Package dependency from FeatureServices where unused. Updated BalanceService to always attempt fetching earn balances (removed the hasEarnBalance guard). These changes centralize/decouple earn balance handling and simplify the EarnService surface.
Replace EarnData with ContractCallData across the codebase and update all usages and mappings. Changes include: - Primitives: remove EarnData, add ContractCallData type and test mock. - TransferDataType and TransactionLoadMetadata: use contractCall (ContractCallData) instead of stakeData/EarnData. - GatewayService, EarnService and mocks: return and propagate ContractCallData. - GemstonePrimitives: rename/migrate mapping extensions from GemEarnData to GemContractCallData and update map() functions. - EthereumSigner: adapt signing logic to use contractCall.callData and contractCall.contractAddress. - Remove unused GemStakeData mapping file and update submodule pointer. These edits align types and mappings to better represent EVM contract call payloads and propagate the rename throughout services, primitives, tests and signers.
534b333 to
4bf93ab
Compare
398912b to
3985b07
Compare
Replace legacy buildBaseInput helpers with a unified buildSigningInput and move signing helpers into a private extension. Introduce buildApprovalInput and signContractCall to centralize contract call and approval logic (handling nonces, gas limits and optional approvals). Replace fatalError usages with throwing AnyError for invalid input types and rename local variables (e.g. base -> signingInput). Remove old buildBaseInputCustom/buildBaseInput implementations and consolidate AnySigner signing logic into a single private sign(...) helper.
Wrap Earn-related UI and logic in #if DEBUG checks so the Earn feature is only available in debug builds. AssetSceneViewModel: showEarnButton and the .earn case in showProviderBalance now return false in non-debug builds. SelectedAssetNavigationStack: EarnNavigationView is shown only under #if DEBUG; otherwise EmptyView is used. This hides experimental/unfinished Earn functionality from non-debug (release) builds.
Apply the APR subtitle style to asset list items in AssetScene so APR subtitles use the correct styling. Normalize asset title formatting to avoid redundant "Name (Name)" when an asset's name equals its symbol. Correct EmptyContentTypeViewModel to use Earn localized strings (title and description) for the .earn case. Mirror the asset title formatting change in TransferAmountCalculatorError as well. Files updated: AssetScene.swift, AssetViewModel.swift, EmptyContentTypeViewModel.swift, TransferAmountCalculatorError.swift.
No description provided.